-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Austenem/CAT-793 unified datasets attribution #3551
Conversation
analysis: Boolean(metadata?.dag_provenance_list), | ||
attribution: creation_action !== 'Central Process' && Boolean(contributors), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's introduce constants for creation_action
.
- Create Dataset Activity
- Central Process
- Multi-Assay Split
- Lab Process
- Create Publication Activity
- External Process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea - I wrote up a type CreationAction
that represents that set of strings. Is that what you had in mind, or would creating individually named constants to represent these strings/an enum be preferable?
} = useProcessedDatasetContext(); | ||
|
||
if (mapped_consortium !== 'HuBMAP') { | ||
if (creation_action !== 'Central Process') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a constant here as well.
dataset: { creation_action, contributors, contacts }, | ||
} = useProcessedDatasetContext(); | ||
|
||
if (creation_action === 'Central Process' || !contributors) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constant here as well.
</LabelledSectionText> | ||
<Stack spacing={1}> | ||
<ProcessedDatasetDescription /> | ||
<LabelledSectionText label="Consortium">{dataset.group_name}</LabelledSectionText> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 'Consortium' the right label for group name? Should group_name
be mapped_consortium
? Do we also want to display the group_name
? @tsliaw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consortium should be the label for mapped_consortium
. I didn't include the group field initially since I assumed that the group should be the same for both the raw and processed, but that's probably not true. @austenem Let's add the group field before the consortium field in the summary section. Can you also replace the consortium field in the mini processed data detail section (the pop-up thing on the right) with group instead? That field should be more useful than consortium.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsliaw Will do! Would it make sense to also replace the "Consortium" field in the summary view top bar, or to add a "Group" field?
context/app/static/js/components/detailPage/ProcessedData/ProcessedDataset/ProcessedDataset.tsx
Outdated
Show resolved
Hide resolved
context/app/static/js/components/detailPage/ProcessedData/ProcessedDataset/ProcessedDataset.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: John Conroy <[email protected]>
…essedDataset/ProcessedDataset.tsx Co-authored-by: John Conroy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for the updates!
Summary
EPICs and Lab Processed datasets should be attributed to the labs that uploaded the dataset. This update removes the "Contact" from the "Summary" section and adds an "Attribution" section for EPICs and Lab Processed datasets.
Design Documentation/Original Tickets
CAT-793 Jira ticket
Figma mockup
Testing
Checked unified views pages with centrally processed and lab processed datasets to ensure sections matched designs.
Lab processed: http://localhost:5001/browse/dataset/225280788b0c66d095d05c4e36a89b81?redirected=True#section-labprocessed-published
Lab & centrally processed: http://localhost:5001/browse/dataset/9046d09943f6a10b70544822de1d5752?redirected=True#section-labprocessed-published
Screenshots/Video
Checklist
CHANGELOG-your-feature-name-here.md
is present in the root directory, describing the change(s) in full sentences.